Skip to content

Conversation

@orestisfl
Copy link
Contributor

Proposed commit message

Switches the openai module to ReportingMetricSetV2WithContext to get rid of a context.TODO().

Taking a look in persistcache.go, I see that we probably don't really need any of the two sync.RWMutexes since the only write-able state is in a sync.Map.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works. Where relevant, I have used the stresstest.sh script to run them under stress conditions and race detector to verify their stability.
  • I have added an entry in ./changelog/fragments using the changelog tool.

How to test this PR locally

Related issues

Switches the openai module to ReportingMetricSetV2WithContext to get rid
of a context.TODO().
@orestisfl orestisfl self-assigned this Oct 29, 2025
@orestisfl orestisfl requested a review from a team as a code owner October 29, 2025 14:19
@orestisfl orestisfl added enhancement cleanup backport-skip Skip notification from the automated backport with mergify Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team skip-changelog labels Oct 29, 2025
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Oct 29, 2025
@github-actions
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

Copy link
Contributor

@stefans-elastic stefans-elastic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@orestisfl
Copy link
Contributor Author

@elastic/obs-infraobs-integrations can I get a review here please?

@orestisfl orestisfl enabled auto-merge (squash) November 4, 2025 15:57
@pierrehilbert
Copy link
Contributor

@lalit-satapathy could we please have someone to take a look here?
cc @ishleenk17

@ishleenk17
Copy link
Member

We are actually not using this module for OpenAI Observability in Integrations. The CEL input is being used.
Let me check how we would want to go about it.
cc: @shmsr

@shmsr
Copy link
Member

shmsr commented Nov 17, 2025

Hi @orestisfl!

We do not support this OpenAI module; in fact, we never actually utilized it. Our initial plan was to use this module for integration, but immediately after we merged it, OpenAI announced their official Usage API. Consequently, we developed the solution in CEL instead. The CEL approach allows us to iterate faster and makes updates easier to deploy.

This module relies on an undocumented API from OpenAI. While that was originally the only way to collect data, it is now obsolete. Since we have the official API, we neither use nor support this module. Furthermore, because OpenAI no longer maintains the undocumented endpoint, users should avoid this module entirely.

Therefore, we are no longer accepting changes to this module. I will add a deprecated comment, I will see how to do it and will do the needful. We will only make an exception for changes that are critical blockers for other components. We will not accept any non-blocking changes as this module is no longer maintained.

In case you want to merge as this could be a blocker — like removing some function or use of some package or something of that nature that could be a blocker, please let us know. I will approve those PRs.

@orestisfl orestisfl closed this Nov 17, 2025
auto-merge was automatically disabled November 17, 2025 13:44

Pull request was closed

@orestisfl orestisfl deleted the metricbeat-openai-use-context branch November 17, 2025 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-skip Skip notification from the automated backport with mergify cleanup enhancement skip-changelog Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants